Am I correct in my reading of this that it doesn't do anything to support different types of scores as requested by #313 and instead is just about displaying the current scoring mechanism in a few different ways? Just checking so we know to be sure to keep #313 open if we merge this PR.

The mention of a "banner" also reminds me that we should probably do a better job describing what the banner is in the documentation. A labeled screenshot might be nice.

The bannerShowTime variable makes me thing that it's setting when, and for how long to show the banner, but it looks like the variable actually controls how to format the countdown (or up) timer. I'd propose the following variable name changes:

• bannerEnable - currently showBanner
• bannerTimerMode - currently bannerShowTime
• bannerProgressEnable - currently bannerShowProgress
• bannerScoreEnable - currently bannerShowScore

I recognize this would break some existing configs using showBanner so we might want to add a warning message if showBanner is found in a config.

Correct, this PR isn't linked to/doesn't close #313 as it does not propose a solution for that issue, just cleans up general banner behavior. I believe we should create a separate PR to deal with #313 once we decide on a more formal design for scoring.

I agree that adding a screenshot to document the banner is a good idea. This screen shot should probably include labels of the time, progress, and score fields.

We use the show[X] convention is fairly widely in FPSci config parameter names. The showHUD, showAmmo, showPlayerHealth, showReferenceTarget, showCursor, etc parameters is what showBanner/bannerShow[X] naming was based upon, so if do choose to change this we should likely change everything. I believe the only place we currently use [x]Enable is the logEnable field (which makes sense since it is not a visual element).

I do think it is a good idea to move all of our config fields to names that start with banner if possible, grouping them logically if serialized out. I also agree that bannerTimerMode is an improvement over bannerShowTime and we should make this change. Otherwise I don't have strong preferences on naming for these fields.

Cool, your reasoning makes sense. Let's just switch to bannerTimerMode and keep the other names how they were.

I just tried to run this PR on my experiment config that works with master and other branches, and this one crashes on startup with an access violation in line 181 of Logger.cpp. I can't explain what is going wrong, or why these specific changes would cause this error (on a cursory read of them they shouldn't), but this is going to need more investigation.

@jspjutNV can you provide the exception message and your config file to review?

This exception seems odd since nothing in this branch should impact the logger at all...

Repeating my comment from #319 since it's probably the same bug I'm hitting.

I believe I've gotten it to build by changing line 182 in Logger.cpp to make the first "'" literal into a G3D::String with the following.

		String("'") + sessConfig->id + "'",

Since there's a bunch more of the same implicit conversion to G3D strings to get the G3D + operators to work, it seems like we should probably be explicitly making those strings into G3D::String types. I cannot explain why this only breaks on a couple branches, some of which were working for me before, but doesn't break on the master branch.